@hawkticehurst just a heads up, I'm hitting errors on the lint step locally due to package issues, I think it may be related to peer dependencies.

@chrisdholt, I was able to replicate the PR, and first off, yay/thank you for fixing the build script error that was happening!! 🥳

Secondly, I was also able to replicate the linting error and on my end, it looks like it's specifically coming from the need for peer dependencies that support @microsoft/eslint-config-fast-dna. I think I have a solution and I'd love to see if it works on your end.

Could you install the following:

npm i -D @typescript-eslint/eslint-plugin
npm i -D eslint-plugin-react

After that update the .eslintrc.js file to include an overrides section at the bottom with the following:

overrides: [
  {
    files: ['*.ts'],
    parserOptions: {
      project: ['./tsconfig.eslint.json'],
    },
  },
],

Finally, create a new tsconfig.eslint.json file in the root directory that contains the following:

{
  "extends": "./tsconfig.json",
  "include": ["src"],
  "exclude": ["node_modules"]
}

If you now run npm run lint it should pass without any errors. If that works I'd love if you could push those changes to see if that resolves the CI pipeline issue.

Note: There are still 18 no-unused-vars warnings being reported because some TS types are being treated as variables for some reason, but I'm sure I can go do some snooping to figure out how to correctly configure eslint to not treat those types as variables after this PR is merged.

Oh, it also looks like there's an error when building Storybook (i.e. the toolkit-ci / build-docs pipeline) which I'm assuming is due to the Introduction.stories.mdx file inside docs/storybook.

Could you just delete the storybook directory and file for now and I'll go back and fix that in a later PR? I tried to troubleshoot it for a bit but I think I'll have to open an issue in the Storybook repo and I don't want this to be a blocker for getting this PR merged.

Also updated your PR comment so this will close issue #252 once merged.

Beyond the requests I made to see if we can resolve the CI pipeline build issues, looks great! Thank you again!!

Sounds good - I'll dig into this later and see if we can get this taken care of!

@chrisdholt, I was able to replicate the PR, and first off, yay/thank you for fixing the build script error that was happening!! 🥳

Secondly, I was also able to replicate the linting error and on my end, it looks like it's specifically coming from the need for peer dependencies that support @microsoft/eslint-config-fast-dna. I think I have a solution and I'd love to see if it works on your end.

Could you install the following:

npm i -D @typescript-eslint/eslint-plugin
npm i -D eslint-plugin-react

After that update the .eslintrc.js file to include an overrides section at the bottom with the following:

overrides: [
  {
    files: ['*.ts'],
    parserOptions: {
      project: ['./tsconfig.eslint.json'],
    },
  },
],

Finally, create a new tsconfig.eslint.json file in the root directory that contains the following:

{
  "extends": "./tsconfig.json",
  "include": ["src"],
  "exclude": ["node_modules"]
}

If you now run npm run lint it should pass without any errors. If that works I'd love if you could push those changes to see if that resolves the CI pipeline issue.

Note: There are still 18 no-unused-vars warnings being reported because some TS types are being treated as variables for some reason, but I'm sure I can go do some snooping to figure out how to correctly configure eslint to not treat those types as variables after this PR is merged.

I've done this and locally everything checks out, including the warnings you mention above (though...those vars are used it seems :|). On the PR build though, it seems like this is an error with eslint not being found...

I've done this and locally everything checks out, including the warnings you mention above (though...those vars are used it seems :|). On the PR build though, it seems like this is an error with eslint not being found...

I know, many ughs on the "unused" vars issue 😪...

Also odd, I'll see if I can figure out what's going on with the pipeline then.

Oh, it also looks like there's an error when building Storybook (i.e. the toolkit-ci / build-docs pipeline) which I'm assuming is due to the Introduction.stories.mdx file inside docs/storybook.

Could you just delete the storybook directory and file for now and I'll go back and fix that in a later PR? I tried to troubleshoot it for a bit but I think I'll have to open an issue in the Storybook repo and I don't want this to be a blocker for getting this PR merged.

Also, could you remove the Introduction file mentioned above and push the change to see if it will fix the failing build-docs CI?

Oh, it also looks like there's an error when building Storybook (i.e. the toolkit-ci / build-docs pipeline) which I'm assuming is due to the Introduction.stories.mdx file inside docs/storybook.
Could you just delete the storybook directory and file for now and I'll go back and fix that in a later PR? I tried to troubleshoot it for a bit but I think I'll have to open an issue in the Storybook repo and I don't want this to be a blocker for getting this PR merged.

Also, could you remove the Introduction file mentioned above and push the change to see if it will fix the failing build-docs CI?

For sure, was doing one at a time but will remove :)

Also odd, I'll see if I can figure out what's going on with the pipeline then.

How baffling, looks like I may have at some point accidentally uninstalled eslint as a dev dependency??? (And then I assume that it got cached by the package-lock file).

Regardless, package.json doesn't have the core eslint package installed so this may be as simple as:

npm i -D eslint

And then push those changes.

For sure, was doing one at a time but will remove :)

Ahh doi, makes total sense :)

Looks like Docs has passed, I think the issue is with a missing dependency on @storybook/addon-docs, though that does have steep peer dependency requirements that has to be forced (tested yesterday). I'm good with leaving the file removed or if you like I can revert and force resolve the docs conflicts.

Looks like Docs has passed, I think the issue is with a missing dependency on @storybook/addon-docs, though that does have steep peer dependency requirements that has to be forced (tested yesterday). I'm good with leaving the file removed or if you like I can revert and force resolve the docs conflicts.

Yep, that's the exact problem. The thorny part of this issue in that storybook's guidance is that you don't directly install @storybook/addon-docs, but rather install the @storybook/addon-essentials package which bundles addon-docs along with a couple of other addon packages.

So addon-docs is in fact technically installed and is declared inside the package-lock as living under addon-essentials––which has worked just fine in the past. However, for whatever reason tsc (I think?) is no longer picking up that addon-docs lives under addon-essentials.

Looks like Docs has passed, I think the issue is with a missing dependency on @storybook/addon-docs, though that does have steep peer dependency requirements that has to be forced (tested yesterday). I'm good with leaving the file removed or if you like I can revert and force resolve the docs conflicts.

Yep, that's the exact problem. The thorny part of this issue in that storybook's guidance is that you don't directly install @storybook/addon-docs, but rather install the @storybook/addon-essentials package which bundles addon-docs along with a couple of other addon packages.

So addon-docs is in fact technically installed and is declared inside the package-lock as living under addon-essentials––which has worked just fine in the past. However, for whatever reason tsc (I think?) is no longer picking up that addon-docs lives under addon-essentials.

Ah, super interesting. I'll leave as-is for now. I wonder if the same issue exists with eslint not being found by the build?

Ah, super interesting. I'll leave as-is for now. I wonder if the same issue exists with eslint not being found by the build?

Yeah, it's a really bizarre one so I too am happy to leave it for now. Also, did you see my earlier comment about eslint? I think it got accidentally uninstalled at some point as a dev dependency but was just cached by the package-lock file.

Also odd, I'll see if I can figure out what's going on with the pipeline then.

How baffling, looks like I may have at some point accidentally uninstalled eslint as a dev dependency??? (And then I assume that it got cached by the package-lock file).

Regardless, package.json doesn't have the core eslint package installed so this may be as simple as:

npm i -D eslint

And then push those changes.

^ This comment.

Yay! Seems like that fixed the linting issue, but now the pipeline just failed on a formatting error.

Run the following command, push, and you should be good to go:

npm run fmt:fix

Oh and potentially looks like there's a deprecated prettier rule that may need to be removed?

If you can remove the jsxBracketSameLine rule from .prettierrc that would be appreciated.

Thank you for all the help troubleshooting this! 😅

Yay! Seems like that fixed the linting issue, but now the pipeline just failed on a formatting error.

Run the following command, push, and you should be good to go:

npm run fmt:fix

Whoops, did ya see this comment?

Wooo! 🎉🎊 Once again thank you so much for this PR / for helping out on this issue!

Huge thanks and kudos, @chrisdholt!